-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[YAML] Fix simple YAML mappings type hinting #31427
Conversation
Signed-off-by: Jeffrey Kinard <[email protected]>
R: @robertwb |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
# Extract original type from upstream pcoll when doing simple mappings | ||
if input_schema is None: | ||
input_schema = {} | ||
original_type = input_schema.get(transform_name, None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite understanding why you want to use transform_name
as a key in the input schema here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no good reason, I think that was the result of an earlier null issue that no longer applies. Removed a couple lines related to obsolete cases
Signed-off-by: Jeffrey Kinard <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be good to add a test if it's not that hard.
@robertwb Interestingly, if I remove the following from the test, it correctly inferences the types...
i.e. exactly what is done in the ReadFromPubSub which was the IO used in the test case that initially surfaced the error beam/sdks/python/apache_beam/yaml/yaml_io.py Lines 325 to 326 in df067db
|
Signed-off-by: Jeffrey Kinard <[email protected]>
aa7a81b
to
122b032
Compare
Signed-off-by: Jeffrey Kinard <[email protected]>
Fixes #31434 |
There have been type inferencing issues with Beam YAML
MapToFields
where the type(s) from the upstream pcollection are dropped when thebeam.Select
is used on unmapped fields in theMapToFields
expansion (i.e. when usingappend: true
, the unmapped fields are mapped to AnyType).The fix is to register type hints for the implicitly appended fields.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.